Skip to content

Tighten pr-reviewer autofix boundaries#60

Merged
khaliqgant merged 1 commit into
mainfrom
pr-reviewer-tighten-autofix
Jun 13, 2026
Merged

Tighten pr-reviewer autofix boundaries#60
khaliqgant merged 1 commit into
mainfrom
pr-reviewer-tighten-autofix

Conversation

@kjgbot

@kjgbot kjgbot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Tightens the pr-reviewer persona so agent-relay-code[bot] no longer treats broad logic changes as safe auto-pushes.

Pinned persona/workflow:

  • review/persona.ts
  • review/agent.ts
  • review/README.md

rg "apply pr-reviewer fixes|pr-reviewer|agent-relay-code|autofix|auto.?fix|push|no-agent-relay-review" confirmed the active workflow is the review/ persona. Its harness prompt is the cloud-commit boundary: cloud pushes whatever the harness leaves in the PR worktree.

Guardrails Added

  • Auto-edit only lint, formatting, spelling, typo, import-order, or other mechanical non-semantic changes.
  • Use suggestion/comment-only posture for logic, behavior, architecture, and human-judgment changes, especially once a PR already has human review/approval.
  • Never change semantic or safety defaults:
    • no fail-closed to fail-open changes such as timeout/pending/throw/undefined becoming acked/true/{}
    • no truthiness-to-presence swaps
    • no guard default-value edits
  • Never touch lifecycle, termination, reaper, in-flight, dispatch, broker ownership, or process-cleanup code.
  • Never add or modify tests to make the bot's own change pass; test changes are a human-authored decision.
  • Keep honoring no-agent-relay-review.

Evidence For This Tightening

  • factory-sdk #266 / e76232a: changed record.fixtureFiles ? ... to Object.prototype.hasOwnProperty.call(record, 'fixtureFiles') ? ... in fleet.ts loadConfig. That truthiness-to-presence swap re-selected FakeMount for fixtureFiles: null / {} and reintroduced the pulled:0 inert-binary bug V2FIX-10b fixed. This was a fail-open regression.
  • factory-sdk #264 / c77a088: added a per-issue prune of #dispatchFailureReaperHandoffs on dispatch success. That was wrong scope/timing and orphaned in-flight crash protection; the safe version belongs in 11c as per-pair/on-confirmed-dead.
  • All five drive-bys from this session added .test.ts assertions for their new behavior (+53/+29/+17 lines observed), making CI green on the bot's own change. Self-justifying tests defeat the gate.
  • June 12 #243: flipped confirmWrite from timeout to acked, another fail-closed to fail-open regression.

Verification

  • npm run test:review passed: 31/31.
  • npx agentworkforce persona compile review/persona.ts passed.
  • git diff --check passed.

Repo-wide npm run typecheck and npm test still fail on existing unrelated baseline TypeScript errors across multiple personas/runtime event typings before this patch's tests run.

@kjgbot kjgbot added the no-agent-relay-review Disable the pr-reviewer agent for this PR label Jun 13, 2026
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The review agent's scope is narrowed from general auto-fixing to mechanical-only fixes. Persona, harness prompt, documentation, and tests are updated to enforce and verify that the agent auto-applies only lint/format/typo/import-order changes, leaving logic, safety-critical code, lifecycle paths, and tests as reviewer comments requiring human ownership.

Changes

Review Agent Scope Narrowing

Layer / File(s) Summary
Agent scope definition and configuration
review/persona.ts, review/agent.ts, review/README.md
Persona description, REVIEW_AUTHORS input, systemPrompt, module comments, handler comment, and README all updated to reflect that the agent now performs only mechanical lint/format/typo/import-order auto-fixes while leaving logic/safety/test/lifecycle changes as comments requiring human ownership.
Harness prompt constraint implementation
review/agent.ts
reviewHarnessPrompt tightened to explicitly forbid semantic/safety-critical auto-edits, require comment-only suggestions when human judgment is needed, clarify post-harness cloud responsibilities, and prohibit adding or modifying tests to justify changes.
Test verification of constraints
tests/review-agent.test.mjs
New test cases added to verify that reviewHarnessPrompt disallows semantic auto-edits, blocks safety-default and lifecycle changes, and prevents test self-justification without human decision.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

  • AgentWorkforce/cloud#1921: Tightens review harness constraints to forbid auto-push and require cloud-controlled commit/CI gating, directly aligning with the issue's request to gate bot pushes when validation checks fail.

Possibly related PRs

  • AgentWorkforce/agents#51: Both PRs modify reviewHarnessPrompt and corresponding tests in review/agent.ts, adding or strengthening constraints on harness auto-edit scope and review output requirements.

Suggested labels

size:L

Poem

🐰 A rabbit once reviewed with care,
But tried to fix what wasn't there.
Now safe and wise, it checks with glee—
Mechanical fixes, comments for thee! 🔧✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely summarizes the main change: tightening the boundaries of what the pr-reviewer agent can auto-fix, which is the core purpose of the entire changeset.
Description check ✅ Passed The description comprehensively explains the changes across all modified files, provides detailed guardrails and evidence for the tightening, and documents verification steps performed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr-reviewer-tighten-autofix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the PR reviewer agent to adopt a more conservative behavior. Instead of proactively fixing all issues, the agent is now restricted to auto-applying only mechanical, non-semantic fixes (such as linting, formatting, typos, and import ordering). It explicitly forbids auto-editing semantic or safety-critical logic, changing safety defaults, modifying lifecycle/termination code, or adding/modifying tests to make its own changes pass. Unit tests have been added to verify these new prompt constraints. I have no feedback to provide on these changes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@khaliqgant khaliqgant merged commit 2eb4098 into main Jun 13, 2026
1 of 2 checks passed
@khaliqgant khaliqgant deleted the pr-reviewer-tighten-autofix branch June 13, 2026 09:11
@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

No PR breakage found in the current checkout, so I left the PR files unchanged.

Addressed comments

  • No bot or reviewer comments were available in .workforce/context.json or other .workforce artifacts, so there were no external review threads to validate or fix.

Advisory Notes

None.

Local validation run:

  • npm install to restore missing dependencies
  • npm test passed: 63/63 tests
  • npm run typecheck passed
  • npm run compile passed; generated ignored persona artifacts were removed afterward

I am not printing READY because I cannot confirm GitHub-side required checks and mergeability from the local artifacts alone.

@agent-relay-code

Copy link
Copy Markdown
Contributor

No source changes were needed. I reviewed the PR diff against the current checkout and found the prompt/docs/persona/test updates consistent with the PR’s mechanical-only auto-fix goal.

Addressed comments

  • coderabbitai[bot]: posted walkthrough plus “review failed / PR closed or merged during review”; no actionable code request, and current checkout validates the described behavior in review/agent.ts:377 and tests/review-agent.test.mjs:166.
  • gemini-code-assist[bot]: explicitly said it had no feedback; no change needed.
  • agent-relay-code[bot]: previous review-only note reported no PR breakage; revalidated current checkout and no fix was needed.

Advisory Notes

None.

Local validation:

  • npm ci completed.
  • npm test passed: 63/63 tests.
  • npm run typecheck passed.
  • npm run compile passed; ignored generated */persona.json artifacts were removed afterward.

I am not printing READY because the connector only exposed the CodeRabbit commit status as successful, and I cannot confirm full GitHub required-check completion or mergeability from the available data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-agent-relay-review Disable the pr-reviewer agent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants